Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(stages): Add SpEL expression for converting canary config name to id. #4030

Merged
merged 5 commits into from
Dec 15, 2020

Conversation

haleyw
Copy link
Contributor

@haleyw haleyw commented Dec 15, 2020

This PR adds a SpEL expression which will take a canary config name and convert it to an ID.

I have written unit tests for this, but haven't tested it end-to-end yet (mostly because I'm not sure how?). Please let me know if there is more you would recommend testing at this point.

…o id.

Add javadocs.

Add javadocs.

feat(stages): Update canManuallySkip to look at stage context and override it on the PipelineStage to enable pipeline stages to be skippable.
Copy link
Member

@robzienert robzienert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haven't tested it end-to-end yet

Not necessary. The unit tests should be fine.

Looks good, aside from some naming things.

Copy link
Contributor

@anotherchrisberry anotherchrisberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@anotherchrisberry anotherchrisberry merged commit ca398bf into spinnaker:master Dec 15, 2020
KathrynLewis pushed a commit to KathrynLewis/orca that referenced this pull request Jan 31, 2021
…o id. (spinnaker#4030)

* feat(stages): Add SpEL expression for converting canary config name to id.

Add javadocs.

Add javadocs.

feat(stages): Update canManuallySkip to look at stage context and override it on the PipelineStage to enable pipeline stages to be skippable.

* Address NIT's from code review feedback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants